feat: discovery-first tool resolution (make Mason optional)#21
Open
charliie-dev wants to merge 71 commits into
Open
feat: discovery-first tool resolution (make Mason optional)#21charliie-dev wants to merge 71 commits into
charliie-dev wants to merge 71 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements a discovery-first tool resolution model so that tools already available on $PATH (system-provided or Mason-provided) are used directly, while Mason becomes an optional installer backend rather than a hard runtime gate. This supports environments like NixOS/FreeBSD where Mason-installed binaries may be unavailable or undesirable.
Changes:
- Add shared helpers for
$PATHprobing, Masonspec.binextraction, and aggregated “missing tools” warnings. - Rework LSP and DAP setup to be driven by desired dependency lists with discovery-first resolution and deferred installs.
- Update Mason bootstrap for formatters/linters to only install when the tool isn’t already on
$PATH, aggregating missing-tool warnings.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| lua/modules/utils/tools.lua | New helper module for executable discovery + aggregated missing-tool warnings. |
| lua/modules/configs/tool/dap/init.lua | Switch DAP setup to discovery-first resolution instead of mason-nvim-dap’s installed-only handler gating. |
| lua/modules/configs/completion/servers/shuck.lua | Update server comment to reflect the new discovery-first resolution model. |
| lua/modules/configs/completion/mason.lua | Only install formatter/linter packages when not already available on $PATH; aggregate missing warnings. |
| lua/modules/configs/completion/mason-lspconfig.lua | Drive LSP setup via discovery-first resolution + deferred installs, using mason-lspconfig mappings. |
| lua/modules/configs/completion/lsp.lua | Remove external_lsp_deps wiring; rely on centralized discovery-first LSP setup. |
| lua/core/settings.lua | Collapse external/system LSP list into unified lsp_deps list with discovery-first semantics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…bugpy the config-time availability check ran the full debugpy cascade, including the blocking vim.system():wait() import probe, on the shared :Dap tick where the resolver validates every dap_deps client — debugging go paid 200-500ms for python's probe whenever the debugpy-adapter shim was absent. the first shimless check now raises without spawning when the raise can actually provision: both mason.nvim and mason-nvim-dap.nvim are lazy-managed with their code on disk, the mapper still derives a package for python, and mason-registry resolves that package — every link pcall-guarded and degrading to the probe path on any drift. the raise is one-shot per process (module-level latch): if provisioning does not produce the shim, every later validates run falls back to the bounded import probe, so installed-but-shimless and mason-less pip setups keep their recovery and a capable system python still passes without a false missing-warning. launch-path semantics (ttl/budget negative cache) are untouched.
vim.uv.cwd() returns nil when the process working directory has been deleted; the "Debug (using venv)" pythonPath concatenated it unchecked and aborted the launch with a concat error. a dead cwd now degrades to the interpreter-name fallback.
package_of called mason_lspconfig.get_mappings() bare while every sibling registry touch in the file is pcall'd; a drifted or missing get_mappings threw into the resolver's outer pcall, error-marking every off-$PATH server with a raw lua error string. the read is now pcall'd and typed down to mappings.lspconfig_to_package being a table, degrading to the empty map — $PATH-only classification with the normal missing/unknown report, re-fetch-while-empty semantics preserved.
attach_registry_events wrapped both registry:on subscriptions in one pcall deriving one flag: a drift throw from the second subscription after the first registered left the flag false, and the next attach registered the install handler a second time — duplicate retry_pending walks on every later install. each subscription now has its own pcall and its own flag: a succeeded half never re-registers, a failed half retries on the next attach, and total failure keeps the status quo. callback bodies unchanged.
the remote-attach branch validated port strictly but passed config.host through untouched, so a non-string or empty host surfaced as an opaque connection failure instead of the clear config-time error the port already gets. host now mirrors the port contract for shape only (hosts are free-form, so no format guessing): nil keeps the 127.0.0.1 default, anything else must be a non-empty string.
mason_root evaluated its sources once: a pre-mason-load first call with $MASON set cached the env value permanently, so the documented higher-priority mason.settings.install_root_dir was never adopted once mason loaded — call timing inverted the priority. worse, a loaded-but- not-set-up mason.settings (mason-registry requires it with the DEFAULTS; only setup() applies a custom root) could outrank $MASON. the settings source is now read live each call, gated on mason.has_setup: presence decides the branch, existence decides the return (a configured-but-uncreated root yields nil, never a fallback to a wrong root). $MASON stays cached as mason_env_root; the data-dir guess and the user-override memo are unchanged.
… call the mason_path_added flag short-circuited before the membership check, so a $PATH overwritten mid-session was never healed and a root that switched once mason.setup applied a different one never got its bin appended. the flag is gone: the exact-entry membership check each call is the idempotence (one plain find per resolve run). append-only on purpose: identical strings in an externally mutable list admit no robust ownership proof, so nothing is ever removed — and per mason PATH mode nothing needs to be (prepend re-orders, append is tail ordering by choice). mason PATH="skip" remains a documented deviation: the append keeps availability classification and bare-name spawns in agreement; honoring skip coherently needs absolute-command configuration in every consumer, tracked as follow-up.
the "never load mason-registry on a provisioned setup" thunk existed as byte-identical copies (code and comment) in the dap resolver spec and in resolve_runtime_tools. tools.default_registry is now the one copy both specs reference, so a change to the load guard cannot drift between consumers. spec.registry semantics unchanged: still a value-or-thunk, still only invoked in phase 2.
run() re-inlined the thunk resolution that session.resolve_registry
already owns, and the inline copy lacked the final table check: a
drifted thunk returning a non-table crashed phase 2 outright
("attempt to index local registry (a boolean value)"), aborting the
whole resolve pass. run() now resolves through the session helper, so
a non-table result degrades to the nil-registry path — a clean
aggregated missing report instead of a crash.
resolve_deps hand-rolled the valid/invalid dep classification with table.maxn although tools.split_dep_names is the one definition of what counts as a valid dep entry and nvim-lint already delegates to it. the container-shape policy stays here (a non-table lsp_deps is dropped whole, exactly as before); invalid entries now land at the immediate batch's tail instead of interleaved — unobservable, the collector sorts its report and classification is per-name.
the "table cmd -> cmd[1] as the probeable binary" extraction existed
five times across server_info and the cache invalidator with varying
outer guards. binary_of_cmd is now the single rule; the self_resolving
branch keeps {} distinct from a function cmd (nil binary without the
flag), byte-equivalent to the old structure.
…aths the mason install-event handler and :ToolsRetry both probed a pending name's declared bare binaries with the same pcall(binaries_of) + find_executable core. pending_bins_available is now that one core; the is_unsettled ownership guards stay per call site because the two paths interleave different extra evidence around the probe, and the evidence order is unchanged.
the type(x) == "string" and x or nil sanitization of untrusted probe/config fields appeared four times (the validates record and the three runtime-tools probe-result fields). str_or_nil is now the single sanitizer; the collector's non-empty-string variant is a different rule and stays inline.
the single-return wrapper (round-3 g8) protected vararg-position callers from split_dep_names' second return, but every live call site truncates fine without it. reader audit at removal time: - repo (rg 'normalize_names' lua/): the wrapper, its M alias, two internal single-assignment calls (find_executable, exepath_or_error), one external call site (mason-lspconfig lsp_deps set, ipairs position, now parenthesized), one comment reference (updated). - fleet harnesses (rg over /tmp/*smoke*/run.lua): zero hits. - user overlay: lua/user/ exists (gitignored surface) and is currently empty — zero files, zero possible readers. callers use split_dep_names directly; the one vararg-position site keeps its truncation via parentheses.
the export has no external runtime caller today; it stays public as the tested contract of the has_setup-gated root-priority semantics, pinned directly by the g4-latch and collector harnesses. the docstring now says so, making the surface self-justifying in source.
the handler rebuilt the 2-element client_modules table on every call only to compare its first entry against load_client_config's winner string. the loader now records user_won at cache-build time, where the module list already exists, and returns the boolean instead — winner had no other consumer.
resolve_remaining's docstring claimed the parity sweep calls the export; the sweep calls the local upvalue — the export is the manual escape hatch and the harness hook, and now says exactly that. the eager_ft_override_modules export gains its test-hook label (read by the ft_override_consistency harness scenario; no runtime reader), so both surfaces are self-justifying in source.
Reattach the attach_registry_events and package_for_binary docstrings to their actual definitions, add the missing is_unsettled field to the ToolCollector class annotation, and update the phase-2 comments for LSP's defer_phase2 adoption. Compress the PATH-mode narrative behind a mason 2.x semantics anchor, note the async-emit immunity on the update handler, and state that default_registry is only resolved once leftovers exist. The mason_root test-hook label now explains WHY it stays public instead of naming harness scenarios.
Rewrite the user_lsp_configs replay comments around the actual ordering guarantee (discovery runs after the user module), fix the stale server_info/invalidate reference, and swap the per-machine verification claim for the documented :h vim.lsp.enable() semantics. Mark SWEEP_DELAY_MS as kept in sync with nvim-lint's parity sweep. shuck's header now records mise as a deliberate choice: Mason ships a shuck package these days, but the $PATH copy wins under discovery-first; the zsh-dialect caveat gains a version anchor so it can expire.
The markdownlint-cli2 override rationale dated from the bun node-shim era; record that mise ships real node now (kept pending re-evaluation) and NOTE the known limitation that the pattern drops col-less findings (e.g. MD012). Correct the shuck comment: actionlint DOES lint run: blocks via its shellcheck pass-through — shuck complements it. Consolidate the golangcilint blocking-load fact onto refresh_linter and point the other two sites at it; the rawset comment no longer cites the removed factory-wrapper machinery.
The prettier --write-on-temp-copy rationale dated from the bun node-shim era; record that mise ships real node now (kept pending re-evaluation). Drop the two comments restating the format_on_save code and the null-ls migration attribution.
Name the user-override materialization as the second mason-nvim-dap load site, correct the upstream default_setup ipairs claim, and point the validate-order notes in every client back at the canonical contract in init.lua. delve: 38697 is the nvim-dap wiki's example port, not a delve default. lldb: the lldb-dap rename landed in LLVM 18, not 15. python: cover all raise sites and drop two boilerplate configuration comments.
palette_overwrite is a name -> palette map, not a list: fix the @type. Soften the nil_ls/nixd/shuck notes into provisioning-preference statements instead of Mason-registry claims that rot as the registry changes, drop the duplicated timeout literal and scheduled-notify rationale, and note the debug keymaps as a dap lazy-load trigger.
The dead block referenced telescope, which left this config in 2026-03.
palette_overwrite is a PARTIAL override merged via tbl_extend(force), but the palette class declares every field required, so the corrected @type made lua_ls flag the empty default with missing-fields. Keep the class annotation (it drives the advertised key completion) and disable that one diagnostic on the assignment; verified with lua-language-server --check that the suppression line must sit directly above the assignment.
The probe runs synchronously on the :Dap* setup tick; a hung interpreter (dead network FS, broken shim) would block the editor indefinitely. wait(5000) SIGKILLs the probe on timeout with exit code 124 (:h SystemObj:wait()), which the existing non-zero check already treats as a failed probe, so resolution falls through to the remaining candidates.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Feasibility POC — RFC ayamir/nvimdots#1293
Implements the "separate tool discovery from tool installation" idea from this comment: Mason becomes one installer backend, not a hard requirement. On FreeBSD/NixOS/etc. — where Mason's prebuilt binaries don't run — system-provided tools are used directly, with no manual symlinking and no startup notification spam.
No new toggle, no OS detection. Behaviour is driven purely by availability.
Resolution model (per tool)
Applied to LSP, DAP, formatters, and linters through one shared loop (
modules/utils/tools.lua). Everything resolvable without an install registers synchronously (lazy-load trigger replay still works); only installs defer.Changes
modules/utils/tools.lua(new)$PATH+ Mason-bin-dir probe, install-then-configure tracking, bin→package reverse lookup, per-subsystem warning aggregator (one message, not N) with a configurable deadline (settings.tool_install_timeout)mason-lspconfig.lua,lsp.lualsp_deps;external_lsp_depsmerged intolsp_deps; function-cmdservers (e.g.jsonls) probed via the Mason package's declared bins; off-$PATHcmds rewritten to absolute paths beforevim.lsp.enable()(covers MasonPATH = "skip")conform.lua,nvim-lint.lua,mason.luamason.lua'sensure_installedloop removed (UI-only now)dap/init.lua,dap/clients/*dap_deps; mason-nvim-dap's private mappings guarded; client configs self-validate and resolve binaries lazily on the spawn path, so remote attach works without local binariessettings.lualsp_deps: absorbsnil_ls,nixd, andshuck— one list, every entry is wanted. Package-less servers (nixd,shuck) are enabled from their$PATHbinary; when absent they join the aggregated warning (the fix is provisioning via home-manager/mise, not Mason)external_lsp_deps: removedformatter_deps/linter_deps: entries are now the subsystem's tool names, matchinglsp_depssemantics (cmakelang→cmake_format,golangci-lint→golangcilint); the Mason package name is derived, not configuredtool_install_timeout(new): deadline before the aggregated warning is flushed despite unsettled installsVerification
stylua+selene+nix flake check, green.lua_lsvia the new resolver, no errors.Known POC limitations
$PATHdetection leans on the Mason package's declared binaries (nvim-dap has no uniform command registry); adapters without a mason-nvim-dap mapping need a client config.